refactor: security, native patterns, storage migration#890
Conversation
… colors with Apple system
…ove iOS row height
datlechin
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e222375c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key in legacyKeys { | ||
| defaults.removeObject(forKey: key) | ||
| } | ||
| defaults.set(true, forKey: migrationCompleteKey) |
There was a problem hiding this comment.
The migration unconditionally deletes every legacy UserDefaults key and marks migration complete even if writing the new JSON files failed earlier (for example, due to disk-full or permission errors). In that failure path, users lose all saved column layouts and the app will not retry migration on next launch because migrationCompleteKey is already set.
Useful? React with 👍 / 👎.
| defaults.removeObject(forKey: key) | ||
| } | ||
|
|
||
| defaults.removeObject(forKey: legacyKnownFilterKeysKey) | ||
| defaults.set(true, forKey: migrationCompleteKey) |
There was a problem hiding this comment.
This migration removes each legacy filter key and sets migrationCompleteKey regardless of whether writing the replacement file succeeded. If file writes fail during first-run migration, users' per-table filters are deleted from UserDefaults and cannot be recovered because future launches skip migration.
Useful? React with 👍 / 👎.
| Task { | ||
| await MCPServerManager.shared.stop() | ||
| NSApp.reply(toApplicationShouldTerminate: true) | ||
| } |
There was a problem hiding this comment.
NSApp.reply(toApplicationShouldTerminate:) is called inside an unannotated Task, which is not guaranteed to execute on the main actor. After await MCPServerManager.shared.stop() the continuation can resume off-main, and calling this AppKit API from a background executor can trigger main-thread violations or quit-flow instability.
Useful? React with 👍 / 👎.
1 participant
Summary
Comprehensive macOS HIG and native patterns audit resulted in fixes across 3 priority batches:
Batch 1 — Security & Stability
JSONSerializationinstead of hand-rolled string interpolation (fixes control character escaping)applicationShouldTerminate+.terminateLaterBatch 2 — HIG / Native Feel
SearchFieldViewwith nativeNativeSearchField(NSSearchField) in keyboard shortcuts, database switcher, and quick switcheronMoveUp/onMoveDownarrow key callbacks toNativeSearchFieldviacontrol(_:textView:doCommandBy:).onTapGesturewithButton+.buttonStyle(.plain)in 8 locations for keyboard/VoiceOver accessibilityBatch 3 — Architecture & i18n
ColumnLayoutStoragefrom UserDefaults to file-based (~/Library/Application Support/TablePro/ColumnLayout/)FilterSettingsStorageper-table data from UserDefaults to file-based (~/Library/Application Support/TablePro/FilterState/)String(localized:))Test plan
~/Library/Application Support/TablePro/ColumnLayout/andFilterState/directories created